Skip to content

Conversation

@brettmc
Copy link
Contributor

@brettmc brettmc commented Mar 27, 2025

  • add types::ArgumentTypeHint and types::ReturnTypeHint
  • create interfaces before classes, so that classes can implement interfaces provided by module
  • [breaking] ReturnType accepts a ReturnTypeHint instead of a TypeInfo, change argument name from type_info to type_hint
  • add API to allow args and return types to be nullable: Argument.allow_null()
  • add API to allow arguments to have a default value: Argument.with_default_value(CString)
  • tests + documentation updates

@brettmc brettmc force-pushed the typehints branch 2 times, most recently from 4d9020f to bfa9e1d Compare March 27, 2025 11:56

// leak Interface so that ClassEntry can be retrieved later, during module
// startup
let i_foo_copy: &'static Interface = Box::leak(Box::new(i_foo));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not happy with this, it's ugly. I wonder whether the whole late-loading-via-functions issue could be removed if we just accepted a String for interface name, and looked it up from globals when needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, you can find the interface through this method: https://docs.rs/phper/latest/phper/classes/struct.ClassEntry.html#method.from_globals

I think we can modify the prototype of implements, something like:

fn implements(&mut self, interface: Interface)  

Then we can think about how to design it better later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created an issue to address this in a separate PR

@brettmc
Copy link
Contributor Author

brettmc commented Mar 27, 2025

I will tackle the <= 8.1 build failures tomorrow.

@jmjoy
Copy link
Member

jmjoy commented Mar 27, 2025

Thank you for your contribution.

}

/// Argument is optional (also see by_<ref|val>_optional)
pub fn optional(mut self) -> Self {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since this is a Breaking Change, we could remove by_ref_optional and by_val_optional and add by_ref() and by_val setters. So something like Argument::new('name').by_ref().optional(), with pass_by_ref:false being the default ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Allright!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want to make this change in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@brettmc brettmc marked this pull request as ready for review March 29, 2025 08:32
brettmc added 2 commits March 30, 2025 10:12
use the Builder pattern to create an Argument or ReturnType, using
methods to change the defaults. A default is to be required, pass-by-val, not-nullable
@jmjoy jmjoy merged commit f3ef58c into phper-framework:master Mar 30, 2025
19 of 22 checks passed
This was referenced Apr 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants